Fix perpetual re-regeneration of projects with up-to-date caches#268
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughSession grouping now uses shared trunk-session helpers, page cache counts are derived from cached session totals, and fallback/session-file paths coalesce synthetic agent session IDs while excluding warmup-only sessions. ChangesSession Coalescing and Cache Accounting
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
claude_code_log/converter.py (1)
2059-2070: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCoalescing/warmup logic is now inlined in three sites — extract a shared helper.
This loop reproduces the exact coalesce-to-trunk + warmup-filter rule already implemented in
compute_session_data(lines 1434-1435) and again in_generate_individual_session_files(lines 2426-2432). Since the bug this PR fixes was caused precisely by these sites diverging, consolidating them into one helper (e.g.collect_trunk_session_ids(messages, warmup_session_ids)) would keep them locked together and prevent future drift.The current logic is correct; this is a maintainability suggestion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@claude_code_log/converter.py` around lines 2059 - 2070, The coalesce-to-trunk plus warmup-filter logic is duplicated across this loop, compute_session_data, and _generate_individual_session_files, so extract it into a shared helper such as collect_trunk_session_ids(messages, warmup_session_ids). Move the shared sessionId normalization and warmup exclusion into that helper, then call it from all three sites so the behavior stays consistent and future changes can’t drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@claude_code_log/converter.py`:
- Around line 2059-2070: The coalesce-to-trunk plus warmup-filter logic is
duplicated across this loop, compute_session_data, and
_generate_individual_session_files, so extract it into a shared helper such as
collect_trunk_session_ids(messages, warmup_session_ids). Move the shared
sessionId normalization and warmup exclusion into that helper, then call it from
all three sites so the behavior stays consistent and future changes can’t drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 969ee6c1-7ed3-40da-8e20-a01794f11f61
📒 Files selected for processing (1)
claude_code_log/converter.py
Running the full generation twice in a row kept "regenerating" the
same projects every run even though nothing changed. Reproduced on a
real 2GB archive copy: 7 projects re-rendered their combined pages
and 2 projects re-flagged one session on every single run, adding
~70s per run. Two root causes, both self-inconsistent staleness
bookkeeping:
1. Paginated combined HTML (reason: message_count_changed).
_generate_paginated_html cached len(page_messages) — the raw
rendered entry count, which includes Summary/AiTitle/Attachment
entries — but is_page_stale() compares that against
SUM(sessions.message_count), computed by compute_session_data,
which skips those types. Any project where the two counting rules
diverge was permanently stale. Fix: cache the sessions-table sum
for the page (same source the check reads), exactly as
last_timestamp already did. The rendered count still feeds the
on-page stats display.
2. Ghost sessions (reason: not_cached). A fork session whose own
messages all deduplicate into the original session survives only
through its agent sidechains ({trunk}#agent-{id} sessionIds).
compute_session_data coalesces those to the trunk and writes a
sessions-table row, but _generate_individual_session_files dropped
agent ids from its render set instead of coalescing — so the
session was never rendered, its html_cache row never written, and
get_stale_sessions flagged it forever. Fix: coalesce with
get_parent_session_id in the render set (generate_session already
accepts {sid}#agent-* entries), and likewise in the pagination
session filter so such sessions claim a page slot instead of their
messages silently vanishing from the combined output.
Both fixes self-heal: the first run after upgrading regenerates the
affected projects once, writes consistent bookkeeping, and every
subsequent run is fully cached. Verified on the archive copy: run 1
regenerated the 9 known offenders, run 2 reported all 63 projects
cached (8.4s total, previously ~80s with 9 spurious regenerations).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3008b9a to
1921c8f
Compare
|
Great! Strangely, I had no idea this could be a bug... even though I had >10 minutes to go for each repeated full generation (I thought that was just my I tried again, 3 successive runs:
So, it's definitely much better, but maybe it would be worth an extra pass at analysis on my end. |
|
I wasn't sure what it was so just gave it to Fable as a goal to prevent it happening, but it's basically anything that messes with the reconciliation of the cache and files because we're filtering some stuff. I now have a sandboxed development env that I'm going to open source soon and pasted my entire ~/.claude/projects directory in there for it to play with and test. Together with the other PR, cached stuff should now take a few seconds to reconcile unless there are some more bugs like this! |
|
Well, these 3 runs and the final ~200s were done with the latest main (e76e1a6), so it's definitely not just a few seconds for me... |
|
Got it, wasn't trying to gaslight you 😅 but with my transcripts it's fast now, so I was just explaining my not very sophisticated process which will need your projects directory |
Running the full generation twice in a row kept "regenerating" the same projects every run even though nothing changed. Reproduced on a real 2GB archive copy: 7 projects re-rendered their combined pages and 2 projects re-flagged one session on every single run, adding ~70s per run. Two root causes, both self-inconsistent staleness bookkeeping:
Paginated combined HTML (reason: message_count_changed). _generate_paginated_html cached len(page_messages) — the raw rendered entry count, which includes Summary/AiTitle/Attachment entries — but is_page_stale() compares that against SUM(sessions.message_count), computed by compute_session_data, which skips those types. Any project where the two counting rules diverge was permanently stale. Fix: cache the sessions-table sum for the page (same source the check reads), exactly as last_timestamp already did. The rendered count still feeds the on-page stats display.
Ghost sessions (reason: not_cached). A fork session whose own messages all deduplicate into the original session survives only through its agent sidechains ({trunk}#agent-{id} sessionIds). compute_session_data coalesces those to the trunk and writes a sessions-table row, but _generate_individual_session_files dropped agent ids from its render set instead of coalescing — so the session was never rendered, its html_cache row never written, and get_stale_sessions flagged it forever. Fix: coalesce with get_parent_session_id in the render set (generate_session already accepts {sid}#agent-* entries), and likewise in the pagination session filter so such sessions claim a page slot instead of their messages silently vanishing from the combined output.
Both fixes self-heal: the first run after upgrading regenerates the affected projects once, writes consistent bookkeeping, and every subsequent run is fully cached. Verified on the archive copy: run 1 regenerated the 9 known offenders, run 2 reported all 63 projects cached (8.4s total, previously ~80s with 9 spurious regenerations).
Summary by CodeRabbit